Skip to content

feat: version list endpoint can filter with CEL expression#1047

Merged
adityachoudhari26 merged 2 commits intomainfrom
dep-version-cel-filtering
Apr 23, 2026
Merged

feat: version list endpoint can filter with CEL expression#1047
adityachoudhari26 merged 2 commits intomainfrom
dep-version-cel-filtering

Conversation

@adityachoudhari26
Copy link
Copy Markdown
Member

@adityachoudhari26 adityachoudhari26 commented Apr 23, 2026

fixes #1046

Summary by CodeRabbit

  • New Features
    • Added optional filtering support to the deployment versions endpoint. Clients can now apply custom filter expressions when querying deployment versions from a specific deployment, providing more granular control over returned results and eliminating the need for multiple API requests or additional client-side processing to achieve desired filtering outcomes.

Copilot AI review requested due to automatic review settings April 23, 2026 16:54
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

📝 Walkthrough

Walkthrough

Adds CEL expression filtering support to the deployment versions list endpoint. When a cel query parameter is provided, the handler validates the CEL expression, fetches candidate versions, evaluates the expression against each candidate in-memory, and returns filtered results. Without cel, the existing database-driven pagination behavior is preserved.

Changes

Cohort / File(s) Summary
OpenAPI Specification
apps/api/openapi/openapi.json, apps/api/openapi/paths/deploymentversions.jsonnet
Adds optional cel query parameter (string, allowReserved: true) to the listDeploymentVersions operation contract.
API Implementation
apps/api/src/routes/v1/workspaces/deployments.ts
Implements CEL filtering logic: validates CEL selector, fetches up to 1000 candidate versions, evaluates expression per candidate, filters results, and returns accurate total count. Preserves DB-driven behavior when cel is absent.
Type Definitions
apps/api/src/types/openapi.ts
Adds optional cel?: string field to listDeploymentVersions query parameters type.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 CEL expressions now filter with grace,
Through deployment versions, each finding its place,
In-memory magic when filters take flight,
A rabbit's delight—order and might! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: adding CEL expression filtering support to the version list endpoint.
Linked Issues check ✅ Passed The PR implements CEL-based filtering for the deployment versions endpoint as required by issue #1046, with proper validation, in-memory evaluation, and pagination handling.
Out of Scope Changes check ✅ Passed All changes are directly related to adding CEL filtering support to the deployment versions endpoint with no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dep-version-cel-filtering

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a cel query parameter to the deployment versions listing endpoint to support filtering results via a CEL expression (fixes #1046).

Changes:

  • Adds cel as an optional query parameter for GET /v1/workspaces/{workspaceId}/deployments/{deploymentId}/versions in OpenAPI types/spec.
  • Implements server-side CEL validation and in-memory filtering of deployment versions using cel-js.
  • Updates the generated OpenAPI JSON output to include the new parameter.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
apps/api/src/types/openapi.ts Adds cel query param typing for listDeploymentVersions.
apps/api/src/routes/v1/workspaces/deployments.ts Validates and applies CEL filtering to deployment versions; changes pagination behavior.
apps/api/openapi/paths/deploymentversions.jsonnet Documents the new cel query parameter for the endpoint.
apps/api/openapi/openapi.json Includes the generated OpenAPI parameter definition for cel.
Comments suppressed due to low confidence (1)

apps/api/src/routes/v1/workspaces/deployments.ts:292

  • The query is only filtered by deploymentId and does not verify that the deployment belongs to the {workspaceId} in the path. This can allow cross-workspace access if a caller knows a deploymentId. Consider joining deploymentVersion to deployment and adding deployment.workspaceId = workspaceId (similar to apps/api/src/routes/v1/workspaces/deployment-versions.ts:18-30).
  const allVersions = await db
    .select()
    .from(schema.deploymentVersion)
    .where(eq(schema.deploymentVersion.deploymentId, deploymentId))
    .orderBy(

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +293 to +298
@@ -283,11 +294,16 @@ const listDeploymentVersions: AsyncTypedHandler<
? asc(schema.deploymentVersion.createdAt)
: desc(schema.deploymentVersion.createdAt),
)
.limit(limit)
.offset(offset);
.limit(1000);

Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.limit(1000) hard-caps the candidate set, so pagination and total become incorrect whenever a deployment has >1000 versions (and offset > 999 will always return an empty page). If this limit is meant as a safety cap, it should be enforced via API validation and reflected in the response/error; otherwise, fetch all versions for accurate totals or implement chunked fetching until offset + limit items are collected and the full filtered total is known.

Copilot uses AI. Check for mistakes.
Comment on lines +299 to +303
const filtered =
cel != null ? filterDeploymentVersions(allVersions, cel) : allVersions;

const total = filtered.length;
const items = filtered.slice(offset, offset + limit);
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When cel is not provided, the handler still loads up to 1000 rows and paginates in memory. This ignores the requested limit/offset at the database level and can add unnecessary DB/network load compared to applying .limit(limit).offset(offset) in the query for the non-CEL case.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
apps/api/src/routes/v1/workspaces/deployments.ts (1)

266-268: CEL receives raw row with Date object instead of the formatted shape.

version here is schema.deploymentVersion.$inferSelect, so deploymentVersion.createdAt is a JS Date and message is string | null (rather than the API's string | undefined). CEL comparisons like deploymentVersion.createdAt > "2026-01-01" or string ops on message will silently fail (caught by the return false above) and give users counter-intuitive results versus what they see in the response body.

Consider evaluating against the same shape the API returns so the CEL contract matches the documented response:

-      return evaluate(cel, { deploymentVersion: version });
+      return evaluate(cel, { deploymentVersion: formatDeploymentVersion(version) });

This also documents implicitly (via formatDeploymentVersion) which fields are queryable. Worth calling out the available fields in the OpenAPI description for the cel param as well.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/routes/v1/workspaces/deployments.ts` around lines 266 - 268, The
filter currently calls evaluate(cel, { deploymentVersion: version }) with the DB
row (Date and nullable message) which breaks CEL expectations; instead pass the
API-shaped object produced by formatDeploymentVersion so fields like createdAt
are formatted and message becomes the API's optional string. Replace the
evaluate call in the versions.filter block to use
formatDeploymentVersion(version) (or a small adapter that also converts message
null→undefined) and update the CEL param OpenAPI description to list which
fields are queryable to match formatDeploymentVersion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/api/src/routes/v1/workspaces/deployments.ts`:
- Around line 317-333: The current flow fetches up to 1000 candidates into
candidates (query on schema.deploymentVersion), runs
filterDeploymentVersions(candidates, cel) and returns total: filtered.length
which can mislead callers; change the response to include a truncated indicator
and the candidate window size so callers know results were capped. Specifically,
after fetching candidates, set a boolean truncated = candidates.length === 1000
and include truncated (and optionally candidateWindow: candidates.length) in the
JSON response alongside items, total, limit, offset; keep using filtered.length
for total but surface the cap so clients can detect truncation. Ensure you
update the handler that constructs the response (the block that calls
filterDeploymentVersions and res.status(200).json(...)) to add these fields and
keep existing fields unchanged.
- Around line 262-273: The current filterDeploymentVersions function swallows
runtime CEL evaluation errors (catch { return false }) making bad selectors
return empty results; change this by performing an up-front evaluation of the
CEL expression (using evaluate) against a representative candidate (e.g., the
first element of versions) in the request handling code (or inside
filterDeploymentVersions) and if evaluate throws, surface a 400 Bad Request (or
throw an error that your route handler converts to 400) with the evaluation
error message; then update filterDeploymentVersions to not silently swallow
errors (remove the empty catch so evaluation errors propagate) so valid
evaluations filter normally while invalid selectors return a clear error instead
of an empty list.

---

Nitpick comments:
In `@apps/api/src/routes/v1/workspaces/deployments.ts`:
- Around line 266-268: The filter currently calls evaluate(cel, {
deploymentVersion: version }) with the DB row (Date and nullable message) which
breaks CEL expectations; instead pass the API-shaped object produced by
formatDeploymentVersion so fields like createdAt are formatted and message
becomes the API's optional string. Replace the evaluate call in the
versions.filter block to use formatDeploymentVersion(version) (or a small
adapter that also converts message null→undefined) and update the CEL param
OpenAPI description to list which fields are queryable to match
formatDeploymentVersion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b361bc37-5548-4b13-9648-a55d087a5728

📥 Commits

Reviewing files that changed from the base of the PR and between 7414446 and 08309e5.

📒 Files selected for processing (4)
  • apps/api/openapi/openapi.json
  • apps/api/openapi/paths/deploymentversions.jsonnet
  • apps/api/src/routes/v1/workspaces/deployments.ts
  • apps/api/src/types/openapi.ts

Comment on lines +262 to +273
function filterDeploymentVersions(
versions: (typeof schema.deploymentVersion.$inferSelect)[],
cel: string,
) {
return versions.filter((version) => {
try {
return evaluate(cel, { deploymentVersion: version });
} catch {
return false;
}
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silent catch { return false } makes invalid CEL expressions indistinguishable from empty results.

validResourceSelector only validates parse-time syntax (via cel-js parse), not runtime evaluation. A syntactically valid expression that references an unknown field, mis-types a comparison, or otherwise throws at evaluate-time will cause every candidate to be dropped and the endpoint will return { items: [], total: 0 } with HTTP 200 — users will have no way to tell their expression is wrong.

Two options, either is fine:

  1. Log the evaluation error at least once per request so operators can diagnose, e.g.:
Proposed change
 function filterDeploymentVersions(
   versions: (typeof schema.deploymentVersion.$inferSelect)[],
   cel: string,
 ) {
+  let loggedError = false;
   return versions.filter((version) => {
     try {
       return evaluate(cel, { deploymentVersion: version });
-    } catch {
+    } catch (err) {
+      if (!loggedError) {
+        console.warn("CEL evaluation failed for deployment version", {
+          cel,
+          versionId: version.id,
+          error: err,
+        });
+        loggedError = true;
+      }
       return false;
     }
   });
 }
  1. Evaluate once up-front against a representative candidate and reject the whole request with a 400 if it throws, so the caller gets actionable feedback instead of an empty list.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function filterDeploymentVersions(
versions: (typeof schema.deploymentVersion.$inferSelect)[],
cel: string,
) {
return versions.filter((version) => {
try {
return evaluate(cel, { deploymentVersion: version });
} catch {
return false;
}
});
}
function filterDeploymentVersions(
versions: (typeof schema.deploymentVersion.$inferSelect)[],
cel: string,
) {
let loggedError = false;
return versions.filter((version) => {
try {
return evaluate(cel, { deploymentVersion: version });
} catch (err) {
if (!loggedError) {
console.warn("CEL evaluation failed for deployment version", {
cel,
versionId: version.id,
error: err,
});
loggedError = true;
}
return false;
}
});
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/routes/v1/workspaces/deployments.ts` around lines 262 - 273, The
current filterDeploymentVersions function swallows runtime CEL evaluation errors
(catch { return false }) making bad selectors return empty results; change this
by performing an up-front evaluation of the CEL expression (using evaluate)
against a representative candidate (e.g., the first element of versions) in the
request handling code (or inside filterDeploymentVersions) and if evaluate
throws, surface a 400 Bad Request (or throw an error that your route handler
converts to 400) with the evaluation error message; then update
filterDeploymentVersions to not silently swallow errors (remove the empty catch
so evaluation errors propagate) so valid evaluations filter normally while
invalid selectors return a clear error instead of an empty list.

Comment on lines +317 to 333
// CEL is evaluated in-memory, so cap the candidate set to bound cost.
// Filtering applies to the 1000 most-recent (or oldest, for asc) versions.
const candidates = await db
.select()
.from(schema.deploymentVersion)
.where(eq(schema.deploymentVersion.deploymentId, deploymentId))
.orderBy(
order === "asc"
? asc(schema.deploymentVersion.createdAt)
: desc(schema.deploymentVersion.createdAt),
)
.limit(limit)
.offset(offset);
.orderBy(orderBy)
.limit(1000);

const filtered = filterDeploymentVersions(candidates, cel);

res.status(200).json({
items: versions.map(formatDeploymentVersion),
total,
items: filtered.slice(offset, offset + limit).map(formatDeploymentVersion),
total: filtered.length,
limit,
offset,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Hard 1000-candidate cap silently truncates total and pagination.

When cel is supplied, candidates are capped at 1000 before filtering and total is reported as filtered.length. For any deployment with >1000 versions this is misleading in two ways:

  • total no longer reflects the real number of matches — it's bounded by how many of the top-1000 (by createdAt) happen to match. A client seeing total: 37 cannot tell whether there are genuinely 37 matches or 37 within the first 1000 of many thousands.
  • Clients paginating with offset/limit can never reach matches that live outside the 1000-candidate window, so the endpoint will silently omit results.

At minimum, surface the cap to the caller (e.g., an additional flag like truncated: true when candidates.length === 1000, or expose the candidate-window size separately from the match count) and document the behavior on the OpenAPI spec. Longer-term, consider pushing CEL → SQL translation for common predicates, or iterating in DB-ordered chunks until offset + limit post-filter results are found.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/routes/v1/workspaces/deployments.ts` around lines 317 - 333, The
current flow fetches up to 1000 candidates into candidates (query on
schema.deploymentVersion), runs filterDeploymentVersions(candidates, cel) and
returns total: filtered.length which can mislead callers; change the response to
include a truncated indicator and the candidate window size so callers know
results were capped. Specifically, after fetching candidates, set a boolean
truncated = candidates.length === 1000 and include truncated (and optionally
candidateWindow: candidates.length) in the JSON response alongside items, total,
limit, offset; keep using filtered.length for total but surface the cap so
clients can detect truncation. Ensure you update the handler that constructs the
response (the block that calls filterDeploymentVersions and
res.status(200).json(...)) to add these fields and keep existing fields
unchanged.

@adityachoudhari26 adityachoudhari26 merged commit a3fa8f2 into main Apr 23, 2026
12 checks passed
@adityachoudhari26 adityachoudhari26 deleted the dep-version-cel-filtering branch April 23, 2026 17:35
.limit(limit)
.offset(offset);
.orderBy(orderBy)
.limit(1000);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be sufficient? What if you looped pages of 1000 until you got enough results?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: filter deployment versions with cel param

3 participants